Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Contact page fully functional. Utilizes Material UI design components and inherits styles from theme #1330

Merged
merged 38 commits into from
Sep 7, 2022

Conversation

edwinjue
Copy link
Member

@edwinjue edwinjue commented Aug 26, 2022

Fixes #1176

  • Up to date with dev branch
  • Branch name follows guidelines
  • All PR Status checks are successful
  • Peer reviewed and approved

Contact Form (in various states):
1-Initial
2-Invalid
3-Submit
4-Confirmation

Additional toast placements:
Selection_999(2401)
Selection_999(2402)
Selection_999(2403)

Any questions? See the getting started guide

…tion components and react hooks. applied dependent code from v1 into /client/redux/sagas/data.js. confirmed submission works and reaches 311-data github support.
…rm and added line to import styles.scss in client/index.js
… and Icon common components from v1 into @components/common/ and updated Routes.jsx to render Contact component at /contact endpoint
…also applied some vertical spacing between rows
…ion. error message has been implemented if sendGitRequest fails but have not tested this yet. there is still a small delay between the time user clicks submit and when the confirmation, so may need to display a loading animation. removed the /styles folder since none of the .scss files are being used anymore. instead, created a styles folder within components/contact to keep any contact form-related css rules in play (needed for ContactImage). also had to update webpack to include .mjs files from node_modules to get react-toastify to work.
…ctForm component to prevent historical entries from popping up
…splay loading animation while request is being processed
@edwinjue edwinjue linked an issue Aug 26, 2022 that may be closed by this pull request
3 tasks
@edwinjue edwinjue requested a review from nichhk August 26, 2022 22:12
@edwinjue
Copy link
Member Author

in the process of writing unit tests. since this is front-end react code I am planning to use jest/react testing library instead of chai/mocha. if anybody has any experience in this area or suggestions, please feel free to chime in. would love to hear your feedback!

…nore since it was raising the following error for webpack.dev.js and webpack.prod.js: error Unexpected use of file extension js for ./webpack.config.js import/extensions
Copy link
Member

@nichhk nichhk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Edwin, it looks great! Mostly readability comments.

Adding @jekijo as a reviewer since he has React experience as well.

Can you also put a screenshot of the form in the first comment? In general, we don't expect code reviewers to checkout a branch and bring up the frontend themselves.

client/index.js Outdated Show resolved Hide resolved
client/components/contact/ContactForm.jsx Outdated Show resolved Hide resolved
client/components/contact/ContactForm.jsx Outdated Show resolved Hide resolved
client/components/contact/ContactForm.jsx Outdated Show resolved Hide resolved
client/components/contact/ContactForm.jsx Outdated Show resolved Hide resolved
client/components/contact/ContactIntro.jsx Outdated Show resolved Hide resolved
client/components/contact/styles/contactimage.scss Outdated Show resolved Hide resolved
client/components/contact/ContactImage.jsx Outdated Show resolved Hide resolved
client/components/contact/ContactForm.jsx Outdated Show resolved Hide resolved
client/components/contact/styles/contactimage.scss Outdated Show resolved Hide resolved
@nichhk nichhk requested a review from jekijo August 30, 2022 03:57
@edwinjue edwinjue requested a review from nichhk August 30, 2022 08:04
@nichhk nichhk requested a review from melissahermes99 August 31, 2022 21:17
@nichhk
Copy link
Member

nichhk commented Aug 31, 2022

Adding @melissahermes99 in case she has any comments re: the design of the Contact page. Screenshots are in the first comment. Thanks!

Copy link
Member

@nichhk nichhk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Edwin!

client/components/contact/ContactForm.jsx Outdated Show resolved Hide resolved
client/components/contact/ContactForm.jsx Outdated Show resolved Hide resolved
client/components/contact/ContactForm.jsx Outdated Show resolved Hide resolved
client/components/contact/ContactForm.jsx Outdated Show resolved Hide resolved
client/components/contact/ContactForm.jsx Outdated Show resolved Hide resolved
<Grid container alignItems="center" justify="center" direction="column" style={{ gap: '10px' }}>
<Grid container alignItems="center" justify="center" direction="row" spacing={2}>
<Grid item xs={6}>
<TextField
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jekijo or Jordan, any ideas?

client/components/contact/settings/index.js Outdated Show resolved Hide resolved
…to guarantee truthy/falsy values because the variables should always be boolean or bug otherwise
…fails. also changed initial casing in a few comments that begin with method names
…d them via dispatch(reducer(payload)) directly in respective areas
Copy link
Member

@melissahermes99 melissahermes99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reviewing the current Design System and the layouts we've created on Figma, this is definitely what we were hoping for! It looks great. Are we planning to go ahead with the dark mode as default for launch? And how would you feel about relocating the toast? Since we have so much happening on the top half of the screen with the photo backdrop and header, I worry the toast might get lost. However, that being said, I don't think it's the biggest issue because the form resetting also gives the notion that the form has been sent off. Other than those comments, it looks fantastic to me!

@edwinjue
Copy link
Member Author

edwinjue commented Sep 2, 2022

@melissahermes99 thank you so much for taking the time to review and for your feedback!

Regarding the placement of the toast, is there a particular place you have in mind? Moving it won't be much trouble at all. We can try some other areas and if you like, I can take some screenshots for you. Perhaps in the dark area right below the photo backdrop? Please let me know.

Also, you bring up a good point about the form resetting. I will need to run a few more tests to be sure the form is resetting only on a successful submission and not an unsuccessful one. (e.g. - unsuccessful submissions may happen if for some reason the response didn't make it to us due to network issues, etc then a different toast displays with an error icon and a message that reads "We failed to process your message. Please try again later.") Please also confirm this is the behavior you expect.

Regarding the dark mode as default for launch. I briefly heard from @nichhk that there are plans to go with a light theme only but this is something Nich will need to confirm.

Thanks! Looking forward to your response

…ure that the appropriate toast error appears when submitting offline and input fields do not clear. ensured the approriate success toast appears and form fields clear when reconnecting back online and a making a successful submission. also identified a bug in redux/reducers/metadata.js and redux/sagas/metadata.js where when visiting the site offline, console log message upon indicate getMetadataFailure status is undefined. to resolve, i set the value of status explicitly to e and destructured the error accordingly in the reducer.
Copy link
Member

@nichhk nichhk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I'm approving it now. Please merge after addressing the small comments and Melissa's comments.

client/redux/reducers/metadata.js Outdated Show resolved Hide resolved
client/redux/reducers/metadata.js Outdated Show resolved Hide resolved
Copy link
Member

@melissahermes99 melissahermes99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edwinjue A screenshot would be great! I've been looking at the Adobe suggestions for placings toasts and I was wondering if placing it in the bottom right corner might be more effective. My train of thought was that the user would be hitting the submit button on the lower half of the screen and their eyes would remain there for a brief second after submitting. However, because I can't fully visualize what this would look like I am unsure if that would be the right decision either. I do like the idea of including a toast if the form doesn't go through for consistency. That way the response to a successful submission also resembles an unsuccessful submission.

@edwinjue
Copy link
Member Author

edwinjue commented Sep 3, 2022

@melissahermes99 I added a new section in the first comment "Additional toast placements" below the original images. Please take a look and let me know which you like best. I just found out toasts don't like being anywhere else besides around the screen border (top:left/center/right, bottom:left/center/right), so under the cover image is unfortunately out of the question.

By the way, the toasts you are seeing there are the error toasts to give you an idea what they look like.

…etadata.js back to original version on dev branch
Copy link
Member

@melissahermes99 melissahermes99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edwinjue edwin The error toasts look great! Thanks for showing me all the different options, I definitely prefer the bottom right location for the toast to appear. I think everything else looks good to go!

@edwinjue edwinjue merged commit c0a2caf into dev Sep 7, 2022
@edwinjue edwinjue deleted the 1176-auto-create-issues-of-contact-us-form-messages branch September 7, 2022 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto Create Issues of Contact Us Form Messages
3 participants